Skip to content

Conversation

ronjalh
Copy link
Member

@ronjalh ronjalh commented Oct 30, 2026 •

Summary

The menu and the "Your gymnasts" page no longer disagree about who is logged in, "Log in" leads somewhere, and the project has a demo coach that the staff and the students who review it can use to save and delete things. The reasons for the login we have, everything we tried before choosing it, and what a real system would do are written in a new docs/AUTHENTICATION.md.

Closes #136, #138. (#137, accounts with passwords, is closed as not planned: see "Choices".)

What is in it

  • One source of login state. AuthProvider asks the server me once, and useAuth() gives loading, authenticated or anonymous. The navbar, ProtectedRoute and the gymnasts page read it. The isLoggedIn useState in App.tsx is gone.
    • Before, a reload restored the session but the local value started as false, so a logged-in coach was sent away from /gymnasts; and the menu could say "Log out" while the page said "Log in".
    • ProtectedRoute waits while the answer is on its way ("Checking your login…") and does not send anyone away. The menu shows neither "Log in" nor "Log out" until the server has answered.
    • Log in and log out reset the Apollo store (resetStore), which asks me again, so the menu and the page change at once (clearStore only cleared the cache).
  • A login page (/login). "Log in" in the menu goes there, and ProtectedRoute sends a visitor who is not logged in there with the page they wanted; after the login they go to it (only a path on this site; //evil.example is ignored). It has "Log in as the demo coach" (one click) and the form for a test coach of your own. The page says when it is served over http from another computer that it is not encrypted.
  • Delete a gymnast from the gymnasts page, after a confirmation (Base UI alert dialog: focus stays in the dialog, Escape closes it). The dialog has a warning icon, "Delete gymnast", "You’ll permanently lose ’s:" with the list Profile, Routines and List of elements, and the buttons "No, keep it" (first, so Enter does not delete; a 2px edge that is at least 3:1 on the card) and "Yes, delete" (the red of the site). It has the same rounding as the other cards. The page tells the visitor ("… deleted.") and moves the focus to "Add gymnast".
  • The gymnasts page follows the theme (dark mode). It used colors that had no dark value. It now uses theme values (new tokens for the circle, the field, the red and the "No, keep it" button), and a test computes the contrast of each pair from the real values in variables.css.
  • Cards and buttons: bigger cards and text, a smaller circle, and a red trash button that is 48 by 48 CSS pixels (WCAG 2.2 asks for at least 24 by 24 at level AA and 44 by 44 at level AAA).
  • Add form: the field with a save (tick) and a close (cross) at the end of it, as in the inline edit of the Atlassian Design System. No heading and no Cancel button; Enter saves and Escape closes. The label and the field are left-aligned.
  • Name field: at most 70 characters can be typed or pasted (counted as Unicode characters, like the server), and at least 3 are needed to save. The 3 is a rule of the form only; the API still accepts 1–70 (written in docs/API.md).
  • Titles: the page title has the same style as on the Elements page (the same rule in global.css).
  • npm run db:demo makes the demo coach (demo@wag-planner.example) with three gymnasts, each with three elements from the catalog. Run again, it deletes what the demo coach has and puts the example data back; other coaches are never touched.
  • Documentation: docs/AUTHENTICATION.md (new), and the README ("Try the app with the demo coach"), docs/API.md, docs/VM_SETUP.md, docs/UI_PLANS.md and docs/AI_USAGE.md.

Choices

Question Choice Why
Login method The test login (devLogin) and a seeded demo coach Sign in with Google needs https, and the course server has only http. A password would be sent as plain text. The assignment asks for user data that is saved on the server, not for a login.
Passwords? Built and tested first, then dropped (#137 closed as not planned) Over http a password can be read by others on the network. Hashing in the browser makes the hash the password, and an encryption scheme of our own over http is not protected either (the script is sent over http).
Where the login state lives The server's answer to me, through useAuth() One source cannot disagree with itself. It also survives a reload without any saved flag.
The demo coach A normal coach of the test login, with example data It can save and delete like any coach. Everyone who uses it shares the data, which the page says.

docs/AUTHENTICATION.md has the full list of what we looked at (Google, https on the VM, a trusted certificate, passwords, encrypting the password ourselves with a sealed box or OPAQUE, a guest key, passkeys and magic links), what the test login does not protect, and what we would do in production.

Not done (still open)

  • https on the course server. It is not available to the groups, and it did not work on our VM.
  • Sign in with Google (the server side is in the draft PR feat: login with Google #71).
  • The test login does not protect anyone's data: anyone can log in as any test coach. This is written on the login page and in the docs.
  • An access token still works for up to 15 minutes after logout (see "What the server guarantees" in docs/API.md).

Testing

  • npx tsc -b, npx eslint ., npx vite build and npx vitest run pass: 49 test files, 488 tests, none skipped (the backend tests run against a local database).
  • New tests: ProtectedRoute (waits, lets in, sends to the login page with the page they wanted), LoginPage (demo coach, own test coach, from that leaves the site, errors, double click), isInsecureConnection, the delete button, resetDemoCoach (including that the test login finds the same coach), the session functions of the client, and one test of the whole path in src/App.flow.test.tsx: log in, save a gymnast, reload, log out, log in again and see the gymnast, delete it.
  • Existing tests changed because the behaviour changed on purpose: the navbar (no isLoggedIn prop, "Log in" is a link), App.test.tsx (the old test clicked the local toggle; replaced by three tests that use the server's answer), and the gymnast page tests (they need the providers; names are matched exactly, because the delete button has the hidden text "Delete "). No expectation was removed to make a test pass; the test of the login form on the gymnasts page moved to LoginPage.test.tsx.
  • Checked that the tests fail without the behaviour by breaking four things one at a time: waiting while the server has not answered, the same state for the menu and the page, a from that leaves the site, and resetting the store after a login. The last one was not caught by the flow test (it replaces the login functions), so client.test.ts was added, and fails when resetStore is replaced by clearStore.
  • Not tried in a browser by hand in this PR.

🤖 Generated with Claude Code

Ronja added 3 commits October 30, 2026 19:42
…mnast

The menu, the protected pages and the gymnasts page now read the same answer
from the server (me) through useAuth(), so they cannot disagree and a reload
keeps a logged-in coach on the page. Log in goes to a login page with the demo
coach and the test coach; ProtectedRoute waits for the answer and remembers
where the visitor was going. A gymnast can be deleted after a confirmation.

Part of #136, #138
…uld need

Adds docs/AUTHENTICATION.md with what the course asks for, everything we tried
before choosing the test login (Google, https, passwords, our own encryption),
how the login is guarded, what it does not protect, and what we would do with
https. Updates the README, API.md, VM_SETUP.md, UI_PLANS.md and AI_USAGE.md.

Closes #136, #138
@ronjalh ronjalh requested a review from a team as a code owner October 30, 2026 18:43
@ronjalh ronjalh self-assigned this Oct 30, 2026
- Follow the theme: the page, the cards, the form and the login page use
  theme values that have a dark value (new tokens for the circle, the field,
  the red and the "No, keep it" button), and the contrast is tested.
- Bigger cards and text, a smaller circle, and a red trash button that is
  48 by 48 CSS pixels so it can be pressed on a phone.
- Add form: the field with a save tick and a close cross at the end of it,
  no heading and no Cancel button, Enter saves and Escape closes.
- Name: at most 70 characters can be typed or pasted, at least 3 to save.
- The title of the page has the same style as the Elements page.
- Delete dialog: a warning icon, "Delete gymnast", what is lost as a list,
  and "No, keep it" and "Yes, delete" with a visible edge and a shadow.

Refs: #136, #138
@anettkva
Copy link
Member

Really solid work! 🎉 Tested in a clean copy with a seeded database: tsc, lint, the build and all 498 tests pass, and npm run db:demo works.

I also tried it in the browser, since the PR says it was not tried by hand: /gymnasts sends you to the login, the demo coach brings you back with the three gymnasts, adding survives a reload, the delete dialog starts on "No, keep it" and Escape closes it, deleting says "… deleted." and moves the focus to "Add gymnast", dark mode has good contrast, and logging out sends /gymnasts to the login again.

Small things:

  • The login page (and "Checking your login…") has no page title, so the tab says "t29-project-2". With the merge, dev's PageTitle can be added there.
  • Heads-up for the merge: dev's title test in YourGymnastsPage.test.tsx uses findByText(/Emma Berg/), which will also find the hidden "Delete Emma Berg". It needs the exact match you use in your other tests.
  • Everyone shares the demo coach's data, so one reviewer can empty it for the next. Maybe run npm run db:demo on the VM before each review round, or say in the README how to reset it?
  • backend/src/seed/demo.ts fails Prettier.

Generated with Claude Code

Ronja added 12 commits October 30, 2026 20:46
- The save tick is green in both themes (white tick on green in light mode,
  dark tick on green in dark mode) and is no longer the color of the card.
- The hint "3 to 70 characters" is gone. An error shows only after Save or
  Enter, in red: "Name is required" for an empty name, as the API asks for
  1 to 70 characters. No more than 70 can still be typed or pasted.
- The gymnast cards get a tint and a deeper shadow on hover (both themes),
  and a pointer, to show that they can be pressed one day.
- "No, keep it" has white text in dark mode only, on a dark fill with a light
  2px edge (9.6:1 on the card). White text and a fill that is 3:1 on the card
  cannot both be had, so the edge carries the contrast of the control.
- The test of the whole path gets more time when all the tests run at once.

Refs: #136, #138
…dark mode

The outline variant of the button sets colors of its own in dark mode
(dark:bg-input/30), and they won over the green of the save tick and the fill
of "No, keep it", so neither had the intended color in dark mode. Both now use
the default variant, and the hover of the two ghost buttons is set for dark
mode too. A test fails when an outline button does not also set its colors for
dark mode. The test of the whole path answers the new CatalogStructure query.

Refs: #136
The tab said the name of the project on /login and while "Checking your
login…" was shown (WCAG 2.4.2). Both now use PageTitle like the other pages.
The title test of the gymnasts page matches the name exactly, because the
hidden "Delete <name>" would also match a pattern.

Refs: #136
The extra check for 6:1 was a design choice that only pinned the current
color. The limits of the rule stay: 3:1 for the edge of a control and 4.5:1
for the text.

Refs: #136
A blue-grey line (3.6:1 on the card, above the 3:1 of WCAG 1.4.11) does not
draw the eye first, as the lighter line did.

Refs: #136
@anettkva anettkva merged commit 59141a5 into dev Oct 30, 2026
@anettkva anettkva deleted the 136-accounts-login-and-demo-coach branch October 30, 2026 20:42
This was referenced Nov 3, 2026
Sign in to join this conversation on GitHub.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants